Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MusicXML export: Make rests before making notation #1361

Merged
merged 6 commits into from
Aug 8, 2022

Conversation

jacobtylerwalls
Copy link
Member

Follow up to #1242. Didn't quite fix #904 because we need to make rests before making notation (if we are to have the opportunity to re-express the rests inside tuplets).

@jacobtylerwalls jacobtylerwalls marked this pull request as draft August 6, 2022 23:27
Comment on lines -1682 to -1688
>>> v = stream.Voice(note.Note())
>>> m = stream.Measure([meter.TimeSignature(), v])
>>> GEX = musicxml.m21ToXml.GeneralObjectExporter(m)
>>> out = GEX.parse() # out is bytes
>>> outStr = out.decode('utf-8') # now is string
>>> '<note print-object="no" print-spacing="yes">' in outStr
True
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is moved to fromGeneralObject() doctest

Comment on lines +422 to +423
# workaround until getET() helper starts calling fromGeneralObject
s = GeneralObjectExporter().fromGeneralObject(s)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really need to get all the musicxml tests to declare whether or not they depend on notation being made. This was a problem before we added makeNotation=False as an option -- essentially, getET() bypasses part of the normal musicxml export path.

@jacobtylerwalls jacobtylerwalls marked this pull request as ready for review August 7, 2022 15:31
@coveralls
Copy link

coveralls commented Aug 7, 2022

Coverage Status

Coverage decreased (-0.003%) to 93.104% when pulling 127b176 on make-rests-before-notation into de1be4b on master.

@mscuthbert mscuthbert merged commit 1a631f8 into master Aug 8, 2022
@mscuthbert
Copy link
Member

Thanks!

@mscuthbert mscuthbert deleted the make-rests-before-notation branch August 8, 2022 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Express durations with or without tuplets according to context
3 participants